stmmac: add arguments so jl2xx1 can be loaded #13
Conversation
|
Thank you for the PR, by the config.ini it should be possible to add a enable/disable switch in CE settings to easy access the driver. Give us some time to check how it works, thx. |
|
Another idea would be to add a property in Ethernet node in DTS. So if exist and any other value than zero the driver can be loaded. CE have a easy option in CE settings to modify DTS on boot. |
|
That would need some dt handling in |
|
Let me check it later, normally the DTS values should be read on driver probe, so early enough. I will try it when I get chance to get access to a PC. |
|
Please have a try with this patch on top of your PR. It is untested so maybe have some typo or other errors inside. You will need to add the property If you remove the property or set it to diff --git a/Documentation/devicetree/bindings/net/stmmac.txt b/Documentation/devicetree/bindings/net/stmmac.txt
index 41b49e6075f5..015881ef0772 100644
--- a/Documentation/devicetree/bindings/net/stmmac.txt
+++ b/Documentation/devicetree/bindings/net/stmmac.txt
@@ -66,6 +66,9 @@ Optional properties:
MAC HW capability register.
- mdio: with compatible = "snps,dwmac-mdio", create and register mdio bus.
+- JL2XX1:
+ - jl2xx1_delay: add property with a value bigger than zero enable jl2xx1 phy SoC handling, default 1500(ms).
+
Examples:
stmmac_axi_setup: stmmac-axi-config {
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index ea85403ff078..73291b17d614 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -122,12 +122,6 @@ static unsigned int chain_mode;
module_param(chain_mode, int, S_IRUGO);
MODULE_PARM_DESC(chain_mode, "To use chain instead of ring mode");
-#ifdef CONFIG_STMMAC_JL2XX1
-static unsigned int jl2xx1_delay=0;
-module_param(jl2xx1_delay, int, S_IRUGO);
-MODULE_PARM_DESC(jl2xx1_delay, "Specify delay when initializing the DMA engine so JL2XX1 can be correctly identified, in millesecond. 0 or any positive interger in the range of an unsigned integer. Default: 0, for skipping the delay entirely. Suggested: 1500, for waiting 1.5 second");
-#endif
-
static irqreturn_t stmmac_interrupt(int irq, void *dev_id);
#ifdef CONFIG_DEBUG_FS
@@ -1665,8 +1659,8 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
atds = 1;
#ifdef CONFIG_STMMAC_JL2XX1
- if (jl2xx1_delay) {
- msleep(jl2xx1_delay);
+ if (priv->plat->jl2xx1_delay) {
+ msleep(priv->plat->jl2xx1_delay);
}
#endif
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index fab578c079a8..01f5356eb8b9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -49,13 +49,6 @@
#define MII_CSR_CLK_GMAC4_SHIFT 8
#define MII_CSR_CLK_GMAC4_MASK GENMASK(11, 8)
-/* JL2XX1 injection */
-#ifdef CONFIG_STMMAC_JL2XX1
-static unsigned int jl2xx1_inject=0;
-module_param(jl2xx1_inject, int, S_IRUGO);
-MODULE_PARM_DESC(jl2xx1_inject, "Specify whether to inject codes when MII bus is initialized through mdio so JL2XX1 can be correctly started. 0 (default) for no, 1 for yes");
-#endif
-
static int stmmac_mdio_busy_wait(void __iomem *ioaddr, unsigned int mii_addr)
{
unsigned long curr;
@@ -338,7 +331,7 @@ int stmmac_mdio_register(struct net_device *ndev)
}
#ifdef CONFIG_STMMAC_JL2XX1
- if (jl2xx1_inject) {
+ if (priv->plat->jl2xx1_delay) {
stmmac_mdio_write(new_bus,0,31,2627);
stmmac_mdio_write(new_bus,0,25,0x1801);
stmmac_mdio_write(new_bus,0,31,0);
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 13bb24c6ec99..d791a3f56074 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -291,6 +291,11 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
plat->force_sf_dma_mode =
of_property_read_bool(np, "snps,force_sf_dma_mode");
+#ifdef CONFIG_STMMAC_JL2XX1
+ if (of_property_read_u32(np, "jl2xx1_delay", &plat->jl2xx1_delay))
+ plat->jl2xx1_delay = 0;
+#endif
+
/* Set the maxmtu to a default of JUMBO_LEN in case the
* parameter is not present in the device tree.
*/
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index 705840e0438f..31eeabd9ebbf 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -142,5 +142,8 @@ struct plat_stmmacenet_data {
int has_gmac4;
bool tso_en;
int mac_port_sel_speed;
+#ifdef CONFIG_STMMAC_JL2XX1
+ int jl2xx1_delay;
+#endif
};
#endif |
|
@7Ji I tried your previous patch with hard-coded values on the S904X4 HK1 RBOX with the JL2101 and about the only thing it did for me is either the 169. address appeared faster or it passed an incorrect MAC to the DHCP server but that IP is not working. The mdio patch also allows ifconfig eth0 up/down without a resource busy message but when it comes up I see |
|
@emveepee I don't think there should be difference between S905X3 and S905X4's MAC as they are pin-to-pin compatible and the two boxes in my hand even use the same board layout -- meaning their PHY are also placed the same and connected the same. But maybe I'm wrong? @Portisch |
|
So I've tested the patch on several devices (one HK1 BOX with JL2101 as experiment group, one BPi M5 and one HK1 Box X4 with RTL8211F as control group) with native dtb, dtb with added The results are collected in the following table, every colomn is the result of at least 3 continous boots and tested with at least 10 minutes of iperf stress test.
The quick-boot issue above is the previously reported issue for JL2101, it means the ethernet can only work when the booting is slow (typically the first boot after burning the image). Otherwise there's no drivers assigned to the PHY ( Now that the patch works as intended and perfectly, I'll merge the patch into my PR branch and adapt the config option description. |
|
I have to ask about the Maybe something like this? diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 73291b17d614..1790678f8698 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1659,9 +1659,9 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
atds = 1;
#ifdef CONFIG_STMMAC_JL2XX1
- if (priv->plat->jl2xx1_delay) {
- msleep(priv->plat->jl2xx1_delay);
- }
+ pr_info("DEBUG---- PHY ID %08x\n", phydev->phy_id);
+ if (phydev->phy_id == 0x937c4032)
+ msleep(1500);
#endif
ret = priv->hw->dma->reset(priv->ioaddr);
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index 01f5356eb8b9..cd3b891e03ae 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -330,15 +330,6 @@ int stmmac_mdio_register(struct net_device *ndev)
goto bus_register_fail;
}
-#ifdef CONFIG_STMMAC_JL2XX1
- if (priv->plat->jl2xx1_delay) {
- stmmac_mdio_write(new_bus,0,31,2627);
- stmmac_mdio_write(new_bus,0,25,0x1801);
- stmmac_mdio_write(new_bus,0,31,0);
- stmmac_mdio_write(new_bus,0,0,0x8000);
- }
-#endif
-
if (priv->plat->phy_node || mdio_node)
goto bus_register_done;
@@ -397,6 +388,15 @@ int stmmac_mdio_register(struct net_device *ndev)
return -ENODEV;
}
+#ifdef CONFIG_STMMAC_JL2XX1
+ if (phydev->phy_id == 0x937c4032) {
+ stmmac_mdio_write(new_bus,0,31,2627);
+ stmmac_mdio_write(new_bus,0,25,0x1801);
+ stmmac_mdio_write(new_bus,0,31,0);
+ stmmac_mdio_write(new_bus,0,0,0x8000);
+ }
+#endif
+
bus_register_done:
priv->mii = new_bus;After run this patch make a dmesg | paste. I am not sure if the phy id is already known in stmmac_main.c. |
|
Both And it works, phy ID is obtained successfully and driver is assigned correctly Full dmesg log: http://ix.io/45mM So all DT handling can be removed and we can just do the corresponding operations based on PHY ID like this instead. |
|
Yes, wonderfull! |
|
I've done some more tests again to find the MII injection is actually not needed and only the delay is needed, and flashed the box cleanly with a couple of different Android firmwares with full-eMMC clean to make sure the result is irrelevant from previous operations, all work fine |
|
In Kconfig there should be bool selection and not tristate. Like |
This adds optional 1.5s delay to stmmac driver which can be enabled through Kconfig CONFIG_STMMAC_JL2XX1=y which will be applied only when the ethernet PHY ID matches JL2XX1 or JL2101 Co-authored-by: Portisch <hugo.portisch@yahoo.de>
|
@vpeter4 |
|
I think it's ok now, I will make a test image for the forum for other users to test. |
As expected (based on the hardcoded change not working), there is no improvement for me with the this test image This has the boot then I insert my USB ethernet disable eth0 http://ix.io/45tI When I try and enable eth0 as noted aboved without the patch I get resource busy now I get [14336.386214@1]- meson6-dwmac fdc00000.ethernet eth0: fail to init PTP. |
|
@emveepee So this is not a sign of failure The expected output is If the output is empty then this PR does not work at all on your box, but if it's assigned succeesfully then this PR works on your box, it's other things that keep your ethernet down and we have to dig deeper |
It is /sys/bus/mdio_bus/drivers/Generic PHY as you expected |
|
@emveepee
If the official driver solves the problem then we can add it to the user space and loads it according with a udev rule |
|
OK I will go back to the last PR with the the hardcoded timeout and the mdio patch. The mdio timeout is what allows me to use ethtools and up/down the device so I think it is part of the solution. Here is the Android dmesg https://pastebin.com/9pPnJYgu Search fdc0 for some relevant info. Right from the initial load it gets the correct MAC with CE I consistently get a bogus MAC |
|
The unbind/insmod method will only work on images built based on this PR, e.g. Portisch's test image. If you're going back to the last PR you will have to do more manual binding and since JL2xx1 is already in there you need to use
Which one? The timeout or the mdio writes? Or both? |
|
I do get But I can't get eth0 via ifconfig or ethtools. |
|
-1 is very wierd, if done correctly it should be used by 0 modules: That -1 is especially wierd since there should be no other modules depending on jl2xx1 as it is an out-of-tree module, let alone some module depending on it being removed causing a used by -1 bug. Ooof, the HK1 Box X4 is really a strange hardware |
|
The -1 might have happened when I downed eth0 since that generated a segfault. |
|
@emveepee https://drive.google.com/drive/folders/1vgXiphp1fPJ7A6lvTePSKw7WTnRtpPYi?usp=sharing There're both the update tarball and full generic image, both should work. If you can test both that'll be much appreciated There's also additionally a split kernel module jl2xx1.ko, if you want to try the driver without flashing the image, you can do as follows (don't do this on an image without the stmmac_main modification, it could probably soft-lock the native kernel): |
|
Are reported on the CE forum the new tar does not work. I also cannot follow your instructions since on boot it binds to the JL2101 PHY and there is no jl2xx1.ko file |
|
These two images already came with the driver built-in. The instructions are not for these two test images, but for other images built based on this PR as there's no public source to build an image with the driver by yourself. |
|
OK yes with the PR I was already using those steps and could enable JL2101 but it wouldn't work either way. |
|
Strange finding: The first reg value for ethmac for mesonsc2 is different from the ethernet address: It is the base bus address and they should be the same. Maybe the wrong bus value caused some low-level operations to fail? For any other dtbs, the first reg value (bus address) for the ethernet node is always the same as the ethernet node itself's address, this could be faulty @emveepee |
|
@7Ji CoreELEC-Amlogic-ng.arm-19.5-Matrix_nightly_20220802-Generic.img: CoreELEC-Amlogic-ng.arm-19.5-Matrix_devel_20220728001259-Generic.img & sc2_s905x4_4g_1gbit.dtb: CoreELEC-Amlogic-ng.arm-19.5-Matrix_devel_20220728001259-Generic.img & sc2_s905x4_4g_1gbit_address_fix.dtb: |
|
I successfully implemented the JL2xx1 on 4.9, SC2, S905X4 HK1 device. There is some "clean up" of the |
Does anyone know where this part of the patch originally came from, or what these numbers mean? |
|
These register writes come from a patch in Lean's OpenWrt repo. They're written through MAC to PHY, and are essential for the initialisation and identification of the PHY. But I don't know their exact meanings. Mostly it came from the source code they got from JLsemi, without explanation on their exact usage. |
|
Thanks. I was able to get in contact with FAE at JLSemi who sent me some details:
|
|
@tmm1 can you check with the FAE what should be tuned when long cables are used? When I add a cheap tp-link 5 port gigabit switch near by the test device the JL2xxx get an DHCP IP and gigabit is working as well, but with errors. So I guess the SoC will require some fine tuning. |
|
I cannot thank you enough for this fix. This is not related to CoreELEC, but I have a T95 Plus that either came with a RTL6211F or JL2101 ethernet chip. The armbian image I was building worked with the RTL6211F , but with the JL2101 it would not work and would fail to get an IP. I rebuilt the Kernel with just patch applied from the image to change the files stmmac_mdio.c and stmmac_main.c and now I have a working ethernet port regardless of which chip is used. I am beyond stoked!!! |
Different from my previous PR which also tries to bring JL2101 PHY support to the kernel. This only implements essential modification in the stmmac driver, and does not port the JL2XX1 driver, instead it uses the generic driver the JL2101. It also keeps the modifications behind some arguments so other devices won't be affected
This adds two new arguments
jl2xx1_delayandjl2xx1_injectto the stmmac driver which can be set through kernel command line, which is then enabled at compilation time through configCONFIG_STMMAC_JL2XX1. It's better to do it here in the kernel instead of splitting to another kmodule like stmmac_jl since stmmac is already set to built-in and can't be unloaded and replaced by another kmodule to be loaded in userspace. Also, by the time stmmac is up and running and recognizes the PHY id it is already impossible to do the early-stage modifications these two arguments do:stmmac.jl2xx_delay, it accepts unsigned interger value, in millesecond. If affects the delay before DMA engine is initialized, so JL2XX1 can be successfully identified.stmmac.jl2xx1_inject, it accepts unsigned integer value, treated as bool. If affects whether or not to write specific code through MDIO before MII bus is initialized, so JL2XX1 can be successfully initialized.The PHY will be driven by the Generic PHY driver instead of the official JL2XX1 driver, this affects less things in the kernel and we don't need to modify the driver upon updates.
The PHY is stressed in a 12hour continous full-speed Tx+Rx iperf3 test, and capped at 720M Tx + 720M Rx when doing dual-direction, and 960M Tx or 960M Rx when doing single-direction, and stayed at an acceptable temp and has an ignorable drop rate during the test.
For end user, the JL2101 support can be enabled through adding
stmmac.jl2xx1_delay=1500 stmmac.jl2xx1_inject=1in the kernel command line. This can either be done with uncommenting the coreelec line in theconfig.iniand adding it in there, or adding a switch option likejl2xx1inconfig.iniand setting these arguments incfgloadif it's set to true.